Extracted ActionPrivileges interface and created RoleBasedActionPrivileges implementation and SubjectBasedActionPrivileges implementation by nibix · Pull Request #5374 · opensearch-project/security

nibix · 2025-06-03T11:38:25Z

Description

This is a refactoring of the ActionPrivileges class: Its interface is decoupled from the implementation. ActionPrivileges is not the interface, RoleBasedActionPrivileges contains the actual implementation.

Additionally, a new class SubjectBasedActionPrivileges was introduced which provides an alternative mode of privilege evaluation; SubjectBasedActionPrivileges does not consider mapped roles. A single instance of SubjectBasedActionPrivileges is meant for a single subject. This is useful for providing privileges for plugins (see #5341 ) and will be also useful for API tokens.

Category: Refactoring
Why these changes are required?: For Create a mechanism for plugins to explicitly declare actions they need to perform with their assigned PluginSubject #5341, we needed a new way to define privileges for plugins. As plugins do not have roles, a decoupling of the RoleBasedActionPrivileges seemed sensible. This principle can be also used for API tokens.
What is the old behavior before changes and new behavior after changes?: No behavioral changes

Testing

New unit test SubjectBasedActionPrivilegesTest

Check List

New functionality includes testing
New functionality has been documented
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

...nTest/java/org/opensearch/security/privileges/actionlevel/RoleBasedActionPrivilegesTest.java

codecov · 2025-06-03T12:25:04Z

Codecov Report

Attention: Patch coverage is 96.32353% with 20 lines in your changes missing coverage. Please review.

Project coverage is 72.37%. Comparing base (9981bc2) to head (7931ab2).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...vileges/actionlevel/RoleBasedActionPrivileges.java	95.37%	7 Missing and 7 partials ⚠️
...earch/security/privileges/PrivilegesEvaluator.java	90.90%	1 Missing and 1 partial ⚠️
...eges/actionlevel/SubjectBasedActionPrivileges.java	98.00%	1 Missing and 1 partial ⚠️
...ensearch/security/securityconf/impl/v7/RoleV7.java	66.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5374      +/-   ##
==========================================
+ Coverage   72.27%   72.37%   +0.10%     
==========================================
  Files         381      384       +3     
  Lines       23714    23819     +105     
  Branches     3655     3669      +14     
==========================================
+ Hits        17139    17239     +100     
- Misses       4772     4779       +7     
+ Partials     1803     1801       -2

Files with missing lines	Coverage Δ
.../opensearch/security/OpenSearchSecurityPlugin.java	`84.57% <100.00%> (-0.02%)`	⬇️
...rity/configuration/SystemIndexSearcherWrapper.java	`91.52% <100.00%> (ø)`
...org/opensearch/security/filter/SecurityFilter.java	`66.51% <ø> (+0.15%)`	⬆️
...ensearch/security/privileges/ActionPrivileges.java	`100.00% <100.00%> (+5.28%)`	⬆️
...g/opensearch/security/privileges/IndexPattern.java	`96.70% <100.00%> (+1.09%)`	⬆️
...curity/privileges/PrivilegesEvaluationContext.java	`97.43% <100.00%> (+0.13%)`	⬆️
...urity/privileges/RestLayerPrivilegesEvaluator.java	`93.75% <100.00%> (ø)`
.../actionlevel/RuntimeOptimizedActionPrivileges.java	`100.00% <100.00%> (ø)`
...urity/privileges/actionlevel/WellKnownActions.java	`85.71% <100.00%> (ø)`
...earch/security/privileges/PrivilegesEvaluator.java	`75.00% <90.90%> (-0.08%)`	⬇️
... and 3 more

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…leges implementation and SubjectBasedActionPrivileges implementation Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java

src/main/java/org/opensearch/security/privileges/actionlevel/RoleBasedActionPrivileges.java

src/main/java/org/opensearch/security/privileges/actionlevel/SubjectBasedActionPrivileges.java

...st/java/org/opensearch/security/privileges/actionlevel/SubjectBasedActionPrivilegesTest.java

Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

Signed-off-by: Nils Bandener <33570290+nibix@users.noreply.github.com>

Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

nibix · 2025-06-11T07:09:25Z

@willyborankin @cwperks I needed to resolve a merge conflict. Can you approve again, please? :)

…leges implementation and SubjectBasedActionPrivileges implementation (opensearch-project#5374) Signed-off-by: Nils Bandener <nils.bandener@eliatra.com> Signed-off-by: Nils Bandener <33570290+nibix@users.noreply.github.com> Co-authored-by: Darshit Chanpura <dchanp@amazon.com> Signed-off-by: Aiden Lindsay <aiden.o.lindsay@gmail.com>

nibix requested review from DarshitChanpura and cwperks as code owners June 3, 2025 11:38

nibix added the skip-changelog label Jun 3, 2025

nibix requested review from RyanL1997, derek-ho, peternied, reta, shikharj05 and willyborankin as code owners June 3, 2025 11:38

nibix commented Jun 3, 2025

View reviewed changes

...nTest/java/org/opensearch/security/privileges/actionlevel/RoleBasedActionPrivilegesTest.java Show resolved Hide resolved

nibix added 5 commits June 3, 2025 14:29

Extracted ActionPrivileges interface and created RoleBasedActionPrivi…

1c54966

…leges implementation and SubjectBasedActionPrivileges implementation Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

Extracted ActionPrivileges interface and created RoleBasedActionPrivi…

1cbc519

…leges implementation and SubjectBasedActionPrivileges implementation Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

Extracted ActionPrivileges interface and created RoleBasedActionPrivi…

7f0a05c

…leges implementation and SubjectBasedActionPrivileges implementation Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

Fix

a787e54

Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

Fix

a7588f6

Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

nibix force-pushed the action-privileges-extracted-interface branch from 4464727 to a7588f6 Compare June 3, 2025 12:30

nibix added 2 commits June 3, 2025 14:45

Fix

15a3597

Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

CLeanup

b4b8f3c

Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

cwperks reviewed Jun 5, 2025

View reviewed changes

src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java Show resolved Hide resolved

cwperks reviewed Jun 5, 2025

View reviewed changes

src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java Show resolved Hide resolved

cwperks previously approved these changes Jun 6, 2025

View reviewed changes

DarshitChanpura reviewed Jun 6, 2025

View reviewed changes

Changes after review remarks

a97ff1b

Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

nibix dismissed cwperks’s stale review via a97ff1b June 7, 2025 05:48

Corrected comments

7b6bb65

Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

cwperks previously approved these changes Jun 9, 2025

View reviewed changes

willyborankin previously approved these changes Jun 9, 2025

View reviewed changes

DarshitChanpura and others added 2 commits June 10, 2025 12:08

Merge branch 'main' into action-privileges-extracted-interface

6a03c02

Merge branch 'main' into action-privileges-extracted-interface

8049328

Signed-off-by: Nils Bandener <33570290+nibix@users.noreply.github.com>

nibix dismissed stale reviews from willyborankin and cwperks via 8049328 June 11, 2025 05:50

Fix

7931ab2

Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

nibix requested review from cwperks and willyborankin June 11, 2025 07:08

willyborankin approved these changes Jun 11, 2025

View reviewed changes

cwperks approved these changes Jun 11, 2025

View reviewed changes

DarshitChanpura approved these changes Jun 11, 2025

View reviewed changes

DarshitChanpura merged commit 1873f02 into opensearch-project:main Jun 11, 2025
72 checks passed

cwperks mentioned this pull request Jun 11, 2025

Create a mechanism for plugins to explicitly declare actions they need to perform with their assigned PluginSubject #5341

Merged

5 tasks

pranu2502 mentioned this pull request Jul 17, 2025

Introduced setting for disabling denormalized privilege data structures #5465

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extracted ActionPrivileges interface and created RoleBasedActionPrivileges implementation and SubjectBasedActionPrivileges implementation#5374

Extracted ActionPrivileges interface and created RoleBasedActionPrivileges implementation and SubjectBasedActionPrivileges implementation#5374
DarshitChanpura merged 12 commits intoopensearch-project:mainfrom
nibix:action-privileges-extracted-interface

nibix commented Jun 3, 2025

Uh oh!

Uh oh!

codecov bot commented Jun 3, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nibix commented Jun 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

nibix commented Jun 3, 2025

Description

Testing

Check List

Uh oh!

Uh oh!

codecov bot commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nibix commented Jun 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Jun 3, 2025 •

edited

Loading